Repository navigation
Conversation
The HMR runtime registered the synthetic bun:wrap module from a hand-written list of eight helpers. The transpiler also emits the TC39 decorator helpers and the private member helpers through bun:wrap, so a decorated class threw "import_bun_wrap.__decoratorStart is not a function" in the dev server. Register the whole runtime.bun namespace instead. The spread keeps the exports as data properties, so a helper read stays one getter hop. Delete the declare module block that nothing imports. Co-authored-by: TiedWithType <TiedWithType@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. WalkthroughThe bake runtime source list now includes additional runtime files. The HMR runtime registers exports from ChangesTC39 Decorators in HMR
Suggested reviewers: Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change makes all runtime helpers available to decorated modules under HMR. No concrete merge-blocking risk was found. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "src/runtime.js", | ||
| "src/runtime.bun.js", |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: Pre-existing sibling of this fix: a developer who edits src/runtime.js still gets a stale bun run helper module after bun bd, because only the bake bundles now track it. The runtime.out.js step in scripts/build/codegen.ts:504-525 lists only src/runtime.bun.js as its input, and the esbuild rule (scripts/build/codegen.ts:144) has no depfile, so an edit to runtime.js (which runtime.bun.js re-exports) never re-runs it. Fix: make every consumer of runtime.js rebuild when it changes, i.e. add resolve(cfg.cwd, "src", "runtime.js") to the inputs of emitRuntimeJs alongside the glob-sources change here, so the bun:wrap module served by bun run (src/runtime/jsc_hooks.rs:3599) is regenerated too.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
A developer changes a helper in src/runtime.js and runs an incremental build. scripts/glob-sources.ts:68-69 now adds src/runtime.js and src/runtime.bun.js to bakeRuntime, so bake.client.js and bake.server.js are regenerated. But emitRuntimeJs at scripts/build/codegen.ts:500-529 declares inputs: [src] where src is only src/runtime.bun.js, and the "esbuild" rule at scripts/build/codegen.ts:144-147 declares no depfile, so ninja does not know runtime.out.js depends on src/runtime.js. runtime.out.js is what src/ast/runtime.rs:21 embeds and what src/runtime/jsc_hooks.rs:3595-3602 serves as the bun:wrap module for bun run. The result is a binary whose dev-server helpers are current but whose bun run helpers are the previous version; tests for bun run behaviour pass or fail against stale code until a clean build. The base branch has the same gap for runtime.out.js and additionally had it for the bake bundles; this PR closes only the bake half of the class.
Verification: src/runtime.bun.js:1 is export * from "./runtime";, so runtime.out.js is a bundle of runtime.js content. scripts/build/codegen.ts:500-509 declares inputs: [src] only, and the esbuild rule at scripts/build/codegen.ts:144-147 sets no depfile/deps. So ninja has no edge from src/runtime.js to runtime.out.js. The base branch already fails the same way for runtime.out.js.
| __name, | ||
| __using, | ||
| } from "../../runtime.bun"; | ||
| import * as runtimeHelpers from "../../runtime.bun"; |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: Browser users of the dev server on engines without SuppressedError get a ReferenceError from __callDispose when a using disposer throws, instead of the synthetic SuppressedError the production build gives. The namespace import at hmr-module.ts:9 pulls __callDispose from src/runtime.bun.js:19-22, which calls new SuppressedError(...) unconditionally; the browser variant at src/bundler/ParseTask.rs:533-534 has a fallback that the client HMR runtime never receives. Fix: build the client bun:wrap from the browser runtime variant (or add the typeof SuppressedError === 'function' fallback to runtime.bun.js's __callDispose) so client and bun build --target browser behave the same.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
If a dev-server client runs in a browser that lacks SuppressedError, a module using using/await using whose disposer throws reaches __callDispose at src/runtime.bun.js:19-23, which does new SuppressedError(e, error, ...) and throws a ReferenceError instead. The production bundler for --target browser uses RUNTIME_USING_OTHER at src/bundler/ParseTask.rs:514-549, whose __callDispose at ParseTask.rs:533-534 checks typeof SuppressedError === 'function' and falls back to a synthesized Error. The client HMR runtime is bundled by src/codegen/bake-codegen.ts:52-66 with target: "browser" but imports the Bun variant via src/runtime/bake/hmr-module.ts:9, so the dev server and the production build disagree. The base branch already imported __using and __callDispose from ../../runtime.bun, so this is pre-existing. Remedy: select the per-target __using/__callDispose when registering bun:wrap on the client, or add the fallback to runtime.bun.js.
Verification: Pre-existing and acknowledged in the PR description. A dev-server client page runs in a browser without a global SuppressedError, and a using/await using disposer throws. src/runtime.bun.js:19-23 defines __callDispose with new SuppressedError(e, error, ...) and no typeof SuppressedError === 'function' guard, unlike src/bundler/ParseTask.rs:533-535. The base already fails by the identical route, so merging makes nothing worse.
|
@robobun will that get merged soon ? can this get into next version ? |
This branch has not been deployed
2 participants
Fixes #44463
Problem
TypeError: import_bun_wrap.__decoratorStart is not a function.bun buildhandles the same file.__decoratorStart,__decorateElement, and the__private*helpers throughhmr.require("bun:wrap"). The HMR runtime built that module from a hand-written list of eight helpers (src/runtime/bake/hmr-module.ts:910).Fix
hmr-module.tsregisters the whole../../runtime.bunnamespace asbun:wrap(32 keys, was 8). The list cannot drift again. The spread keeps data properties, so a helper read stays one getter hop. The deaddeclare module "bun:wrap"block goes away.scripts/glob-sources.tsaddssrc/runtime.jsandsrc/runtime.bun.jsto the inputs of the bake codegen step, so a helper change regeneratesbake.client.jsandbake.server.js.test/bake/dev/bundle.test.ts, two new tests (client and server, with a hot update), both fail on main. Also rantest/bake/dev-and-prod.test.ts,dev/esm.test.ts,dev/hot.test.ts. Self-reviewed: 3 concerns, 3 addressed.Background
runtime.jsholds the helpers the transpiler emits calls to (Imports::ALLinsrc/ast/runtime.rs, 27 names).bun buildinlines them. The dev server routes them through a synthetic module of the HMR runtime, whichsrc/codegen/bake-codegen.tsbundles at build time.Imports::ALLin codegen: about 1.3 KB of dev-only bytes saved for a new build step.Downsides
bake.client.js+4708 bytes raw (+1699 zstd -19),bake.server.js+4682 raw (+1691 zstd -19),bake.error.js+0 (bake-codegen, main vs this branch). The bytes sit in the binary and go out once per page load in development.__commonJS,__toESM,__esm) are reachable throughbun:wrap, the same as underbun build.Notes
Imports::ALLcoverage after the fix: 26 of 27 names. The missing one is__require.runtime.jsdoes not export it and the dev server rewrites it tohmr.require(pinned bytest/bake/dev-and-prod.test.ts).bake.server.js):fe("bun:wrap",{...te})whereteis the__export(runtime_exports, {...})namespace with 32 getters. The spread runs once at runtime load.hmr-runtime-error.tsdoes not importhmr-module.ts, sobake.error.jsis unchanged.__usingand__callDisposefromruntime.bun.js.